moonshine: 0.13.5 -> 0.14.5 - #546531

✅ nixpkgs-review result

Generated using nixpkgs-review-gha (def992f)

Command: nixpkgs-review pr 546531
Commit: 3eea1292093c0543379b0259324429668e2e9470 (subsequent changes)
Merge: 6f77e5e91b9f054a1db95293ff08aaaff5b6f4d2

Triggered by @coolcuber (35880825)
Logs: https://github.com/coolcuber/nixpkgs-review-gha/actions/runs/30365322722/attempts/1

x86_64-linux

aarch64-linux

aarch64-darwin

✅ No rebuilds

Thanks! Patch looks good and is working fine locally, but it's not a requirement to add yourself to the maintainer list. It would make more sense to add yourself after having done more substantial changes.

Also note there's a weekly auto-update on this package, which is generally a bit easier to review and merge than manual version bumps.

Thanks for the review! On the maintainer entry, the weekly auto-update is actually the main reason I'd like to be listed. I add myself as a maintainer to packages I use so that I can personally see to getting the r-ryantm PRs merged sooner.

As far as I know there's no prerequisite of prior substantial changes for adopting a package in pkgs/by-name, but let me know if I've missed something. For context, I reviewed your original PR #544596 adding this package, so I'm not picking it up at random.

If there's nothing you'd like changed in the patch itself, could you dismiss the review so this can get merged?

Ah, I didn't notice the context of the original review.

My personal opinion is that maintainers should at minimum test package binaries before submitting updates, which hasn't happened here. Adding new maintainers just so PRs land a day earlier doesn't seem very effective (especially if it comes at the cost of not testing anything).

As you mention I don't think there are strictly documented requirements for becoming a maintainer, but since I'm not yet familiar with the soft conventions around nixpkgs maintainership I'll defer to my intuition there. My review doesn't block things though, so if other people are happy to add you to the list I don't mind.

0.14.1 was released earlier today, it's probably a good idea to update this PR

✅ nixpkgs-review result

Generated using nixpkgs-review-gha (d38b08f)

Command: nixpkgs-review pr 546531
Commit: 2287ce37f54e5f707cf87cf3ae8310c075ef2161 (subsequent changes)
Merge: 1373ea79d72fcbc9df6769fc832e96d897c8a241

Logs: https://github.com/ap-1/nixpkgs-review-gha/actions/runs/30474225361/attempts/1

x86_64-linux

aarch64-linux

Ah, I didn't notice the context of the original review.

My personal opinion is that maintainers should at minimum test package binaries before submitting updates, which hasn't happened here. Adding new maintainers just so PRs land a day earlier doesn't seem very effective (especially if it comes at the cost of not testing anything).

As you mention I don't think there are strictly documented requirements for becoming a maintainer, but since I'm not yet familiar with the soft conventions around nixpkgs maintainership I'll defer to my intuition there. My review doesn't block things though, so if other people are happy to add you to the list I don't mind.

I did build and run it locally when I opened the PR, but this time I tested game discovery and streaming as well. I avoid ticking the "Tested basic functionality" checkbox when it's unclear what the extent of "basic functionality" is.

maintainers/README.md encourages people who care about a package to assign themselves, no bar of prior work, and there's a documented process for dropping maintainers who go unresponsive. Being listed lets me use nixpkgs-merge-bot on the package, which is more the point than review latency.

Most of my commits on packages I maintain aren't just bumps anyway, and 0.14.1 isn't one either, see the PR body re: polkit. I'm happy to gate it if you'd rather.

If you don't have comments on the patch itself, could you dismiss the review? Requested changes tend to stop committers from picking a PR up even when the reviewer isn't the one merging.

Separately, at some point we should open an issue upstream about dropping their nix/package.nix. There's a fair amount of extraneous config in it that we already handle correctly here.

we should open an issue upstream

probably invite the author/whoever maintains their flake/package to be a maintainer here

same for the moonshine module #544393